-
Notifications
You must be signed in to change notification settings - Fork 0
Update baton-sdk to v0.2.51 #24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Implement new required funcs for AccountManager and CredentialManager interfaces
WalkthroughThe pull request includes updates to the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
pkg/connector/role.go (2)
331-336: LGTM: Consistent implementation of CreateAccountCapabilityDetailsThe implementation aligns with RotateCapabilityDetails, maintaining consistency in credential management approach.
Consider adding a comment explaining why random password is the only supported option for PostgreSQL accounts.
Line range hint
347-359: Consider using specific error type checkingThe error handling for existing users is improved, but consider checking for a specific error type instead of just
err == nilto ensure we're actually handling a "user exists" case and not masking other potential errors.- roleModel, err := r.client.GetRoleByName(ctx, accountInfo.GetLogin()) - if err == nil { + roleModel, err := r.client.GetRoleByName(ctx, accountInfo.GetLogin()) + if err != nil { + if !postgres.IsNotExistsError(err) { + return nil, nil, nil, err + } + } else {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (11)
go.sumis excluded by!**/*.sumand included by nonevendor/github.com/conductorone/baton-sdk/pb/c1/connector/v2/connector.pb.gois excluded by!**/*.pb.go,!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pb/c1/connector/v2/connector.pb.validate.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pb/c1/connector/v2/resource.pb.gois excluded by!**/*.pb.go,!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pb/c1/connector/v2/resource.pb.validate.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/connectorbuilder/connectorbuilder.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/dotc1z/grants.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/sdk/version.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/uhttp/gocache.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/uhttp/wrapper.gois excluded by!vendor/**and included by nonevendor/modules.txtis excluded by!vendor/**and included by none
📒 Files selected for processing (2)
go.mod(1 hunks)pkg/connector/role.go(2 hunks)
🔇 Additional comments (3)
go.mod (1)
8-8: Verify baton-sdk version compatibility
The update to v0.2.51 aligns with the PR objectives to implement new interface requirements.
✅ Verification successful
Version update to v0.2.51 is appropriate and stable
The verification confirms that v0.2.51 is currently the latest stable release of baton-sdk. The changelog between v0.2.47 and v0.2.51 shows important improvements including:
- Network error handling and JSON response improvements
- Performance optimizations for grant expansion
- Memory usage fixes for cache implementation
- Account provisioning capabilities with credential details
These changes, particularly the account provisioning updates, align well with interface requirements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check latest version of baton-sdk and its changelog
# Expected: Confirm v0.2.51 is stable and contains the required interface changes
# Get latest version
gh api repos/conductorone/baton-sdk/releases/latest --jq .tag_name
# Check changelog or commits between v0.2.47 and v0.2.51
gh api repos/conductorone/baton-sdk/compare/v0.2.47...v0.2.51 --jq '.commits[].commit.message'
Length of output: 1444
pkg/connector/role.go (2)
284-289: LGTM: Appropriate implementation of RotateCapabilityDetails
The implementation correctly specifies random password as both supported and preferred credential option, which is appropriate for PostgreSQL role credentials.
Line range hint 284-389: Verify password generation security
The code uses the SDK's crypto.GeneratePassword function. Let's verify its security parameters.
✅ Verification successful
Password generation implementation is secure
The implementation uses strong security practices:
- Uses crypto/rand for cryptographically secure random number generation
- Enforces minimum password length of 8 characters
- Includes a comprehensive character set with:
- Uppercase letters (A-Z)
- Lowercase letters (a-z)
- Numbers (0-9)
- Special characters
- Proper error handling for invalid inputs
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check password generation implementation in baton-sdk
# Expected: Confirm secure password generation parameters
ast-grep --pattern 'func GeneratePassword($_) {
$$$
}'
Length of output: 58
Script:
#!/bin/bash
# Let's try searching for the crypto package and password generation
rg -A 5 "GeneratePassword"
Length of output: 1225
Script:
#!/bin/bash
# Let's check the implementation of GenerateRandomPassword
rg -A 15 "func GenerateRandomPassword" vendor/github.com/conductorone/baton-sdk/pkg/crypto/password.go
Length of output: 713
Script:
#!/bin/bash
# Let's check what characters are used for password generation
rg -B 5 "var characters" vendor/github.com/conductorone/baton-sdk/pkg/crypto/password.go
Length of output: 90
Script:
#!/bin/bash
# Let's try to see the entire password.go file content
cat vendor/github.com/conductorone/baton-sdk/pkg/crypto/password.go
Length of output: 1541
Implement new required funcs for AccountManager and CredentialManager interfaces
Summary by CodeRabbit
New Features
Bug Fixes
Chores
github.com/conductorone/baton-sdk.